Closed Bug 1405141 Opened 8 years ago Closed 8 years ago

Extend test verification to web-platform-tests

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

jgraham points out that wpt is an important omission for test-verify. I had originally thought that wpt was not high value because those tests are often modified outside of mozilla integration, then verified and integrated en-masse, but web-platform tests are, of course, also updated at Mozilla and pushed to hg, just like other test suites. We should try to support them like other test types.
Pushed a couple of lightly-tested commits for the internals of this to mozreview. Please consider this more like f? than r?; I don't expect the patches to land as-is, but I thought it might speed things up here if I did the first cut at the harness-internal parts. Do you have time to look at the try integration and check that my implementation more or less matches the other harnesses?
Flags: needinfo?(gbrown)
Thanks much James, this is a great help. I'm happy to implement the taskcluster/mozharness parts and do integration testing and I should be able to look at that soon. I'll make a comment or two on your patches tonight, then have a closer look tomorrow.
Flags: needinfo?(gbrown)
Comment on attachment 8918847 [details] Bug 1405141 - Add support for rerunning web-platform-tests without restarting, https://reviewboard.mozilla.org/r/189714/#review195142 ::: testing/web-platform/tests/tools/wptrunner/wptrunner/wptcommandline.py:99 (Diff revision 1) > help="run under a debugger, e.g. gdb or valgrind") > debugging_group.add_argument('--debugger-args', help="arguments to the debugger") > + debugging_group.add_argument("--rerun", action="store", type=int, default=1, > + help="Number of times to re run each test without restarts") > debugging_group.add_argument("--repeat", action="store", type=int, default=1, > - help="Number of times to run the tests") > + help="Number of times to run the tests, restarting between each run") For mochitest and reftest, --repeat means re-run the test without restarts (in one browser session), so I'm a little concerned that this could be confusing. Does repeat-until-unexpected restart the browser between runs?
Comment on attachment 8918848 [details] Bug 1405141 - Add stability checking to wptrunner, https://reviewboard.mozilla.org/r/189716/#review195146 ::: testing/web-platform/tests/tools/wptrunner/wptrunner/stability.py:209 (Diff revision 1) > + with open("raw.log", "rb") as log: > + results, inconsistent = process_results(log, iterations) > + return results, inconsistent, iterations > + > + > +def get_steps(logger, repeat_loop, repeat_restart, kwargs_extras): I like "repeat_loop" / "repeat_restart" -- nice and explicit. ::: testing/web-platform/tests/tools/wptrunner/wptrunner/stability.py:246 (Diff revision 1) > + log('::: Test verification %s' % final_result) > + > + logger.info(':::') > + > + > +def check_stability(logger, repeat_loop=10, repeat_restart=5, chaos_mode=True, max_time=None, Those 10 and 5 counts are consistent with mochitest and reftest --verify support and probably a reasonable choice. For what it's worth, I don't think it is necessary to keep steps or repeat counts consistent across test suites / harnesses. It is fine if they are, and that makes things easy to remember, but don't feel constrained to those defaults.
Comment on attachment 8918847 [details] Bug 1405141 - Add support for rerunning web-platform-tests without restarting, https://reviewboard.mozilla.org/r/189714/#review196938 Testing locally, it seems --rerun doesn't always work. If I run mach wpt netinfo/netinfo-basics.html --rerun 5 the test seems to run once and then pause, with console output: Thread-TestrunnerManager-1 INFO Pausing until the browser exits If I manually close the browser, I get: 0:20.13 LOG: Thread-TestrunnerManager-1 ERROR Traceback (most recent call last): File "/home/gbrown/src/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 352, in run new_state = self.wait_event() File "/home/gbrown/src/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 426, in wait_event return f(*data) File "/home/gbrown/src/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 596, in wait_finished return self.after_test_end(True) TypeError: after_test_end() takes exactly 3 arguments (2 given) 0:20.16 LOG: Thread-TestrunnerManager-1 INFO Browser exited with return code -15
Attachment #8918847 - Flags: review?(gbrown)
Comment on attachment 8918848 [details] Bug 1405141 - Add stability checking to wptrunner, https://reviewboard.mozilla.org/r/189716/#review196940 This works great for me. You noted "More work is needed to avoid duplicating all the code" -- did you want to tackle that now? ::: testing/web-platform/tests/tools/wptrunner/wptrunner/stability.py:178 (Diff revision 2) > + if restart_after_iteration: > + kwargs["repeat"] = iterations > + else: > + kwargs["rerun"] = iterations > + > + kwargs["pause_after_test"] = False Maybe pause_after_test = False should be part of --rerun?
Attachment #8918848 - Flags: review?(gbrown) → review+
BTW, I am making (slow) progress on integration with the taskcluster job, particularly determining if a modified file is a web-platform test.
I just realized that wpt has its own mozharness script, rather than running from desktop_unittest. That probably means we need a separate task: Something like TV (verifies mochitest, reftest, xpcshell) and TV-wpt (verifies wpt).
Any modification under testing/web-platform/tests or testing/web-platform/mozilla/tests is a wpt test modification. Upstream has a script that examines git history to determine which files were modified and which tests are affected by the modification. The affected tests part in particular might be good to reuse here; we could modify it to take a list of changes that we get from mercurial rather than git, perhaps. I don't know how much work that is, but I could look at it. In terms of jobs, making this separate from other TV jobs probably does make sense from an implementation point of view. I sort of assumed that there was one of these jobs per testsuite anyway. The code duplication is mostly with other upstream tools and I think it makes sense to fix that in an upstream PR since it probably wants non-Mozilla reviewers. So I'll do that there.
Comment on attachment 8918847 [details] Bug 1405141 - Add support for rerunning web-platform-tests without restarting, https://reviewboard.mozilla.org/r/189714/#review195142 > For mochitest and reftest, --repeat means re-run the test without restarts (in one browser session), so I'm a little concerned that this could be confusing. > > Does repeat-until-unexpected restart the browser between runs? I am a little loathe to change the established meaning in wpt at this time, since upstream and third-party tools depend on it. I might revisit that later if this is a problem and I can fix all the dependencies.
Comment on attachment 8918847 [details] Bug 1405141 - Add support for rerunning web-platform-tests without restarting, https://reviewboard.mozilla.org/r/189714/#review197390
Attachment #8918847 - Flags: review?(gbrown) → review+
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/autoland/rev/3b0ece162b19 Add support for rerunning web-platform-tests without restarting, r=gbrown https://hg.mozilla.org/integration/autoland/rev/97a964bf7f99 Add stability checking to wptrunner, r=gbrown
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/mozilla-inbound/rev/0faad69400fa Add support for rerunning web-platform-tests without restarting, r=gbrown https://hg.mozilla.org/integration/mozilla-inbound/rev/00bec72d17c4 Add stability checking to wptrunner, r=gbrown
Blocks: 1411660
Blocks: 1413729
Blocks: 1417230
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: